Add Hubbard corrections and DFT+U #1158
Conversation
Technici4n
left a comment
Technici4n
left a comment
There was a problem hiding this comment.
I haven't looked at the math yet, but have some comments already
test/hamiltonian_consistency.jl
Outdated
| test_consistency_term(ExternalFromFourier(X -> abs(norm(X)))) | ||
| test_consistency_term(LocalNonlinearity(ρ -> ρ^2)) | ||
| test_consistency_term(Hartree()) | ||
| test_consistency_term(Hubbard(DFTK.OrbitalManifold(;species=:Si, label="3S"), 1.0)) |
There was a problem hiding this comment.
| test_consistency_term(Hubbard(DFTK.OrbitalManifold(;species=:Si, label="3S"), 1.0)) | |
| test_consistency_term(Hubbard(DFTK.OrbitalManifold(; label="3D"), 1.0)) |
Spacing, and let's use P to test multiple m quantum numbers. And does it work if species is removed as well?
There was a problem hiding this comment.
No it doesn't work without species, should it? The intended way should be (in the multiU update that we'll make after this pr is resolved) to add each of the manifolds one by one (so to get a 3d manifold you explicitly ask for all of the single atomic manifolds having 3d orbitals)
- New handling of measure unit for U value; - Hubbard matrix has been renamed everywhere as "nhubbard"; - Function "compute_hubbard_nIJ" has been renamed as "compute_nhubbard"; - New documentation for "compute_nhubbard" function; - New documentation for "Hubbard" struct; - New test on Wigner matrices added in tests/hubbard.jl; - Refactoring inside apply! for HubbardUOperator; - Several lines have been shortened for compactness; - Unutilized functions have been removed from terms/hubbard.jl; - Function reshape_hubbard_proj has been updated.
- New handling of measure unit for U value;
- Hubbard matrix has been renamed everywhere as "nhubbard";
- Function "compute_hubbard_nIJ" has been renamed as "compute_nhubbard";
- New documentation for "compute_nhubbard" function;
- New documentation for "Hubbard" struct;
- New test on Wigner matrices added in tests/hubbard.jl;
- Refactoring inside apply! for HubbardUOperator;
- Several lines have been shortened for compactness;
- Unutilized functions have been removed from terms/hubbard.jl;
- Function reshape_hubbard_proj has been updated.
src/terms/hubbard.jl
Outdated
| U :: Float64 | ||
| end | ||
| function (hubbard::Hubbard)(basis::AbstractBasis) | ||
| isempty(hubbard.U) && return TermNoop() |
There was a problem hiding this comment.
I think it's weird. And it's probably not what you're looking for:
julia> isempty(0)
false
fsicignano
force-pushed
the
hubbard-v2
branch
2 times, most recently
from
15ea5c0 to
053450f
Compare
- Kwarg use_nlcc=false has been added for all meta_gga tests; - Kwarg spin_polarization=:none added to anyonic test, to avoid failure from @Assert length(basis.kpoints) == 1; - Tolerance for condition number in wigner_d_matrix increased to 100, neq decreased to 2*l+2.
fsicignano
force-pushed
the
hubbard-v2
branch
from
053450f to
d0cf01a
Compare
src/terms/hubbard.jl
Outdated
| natoms = length(manifold_atoms) # Number of atoms of the species in the manifold | ||
| nhubbard = Array{Matrix{Complex{T}}}(undef, nspins, natoms, natoms) | ||
| p_I = [Vector{Matrix{Complex{T}}}(undef, natoms) for ik in 1:length(basis.kpoints)] | ||
| # Very low-level, but works |
There was a problem hiding this comment.
Extract l from first label, make sure that all the labels have the same l, and that each block of 2l+1 consecutive labels is the same except for m.
|
@antoine-levitt you said you wanted to have a look, PR is ready! |
|
@antoine-levitt do you still want to have a look or should we go ahead with this PR? |
|
Sorry I didn't see your previous message (got lost in the flood of notifications). I'll take a look! |
antoine-levitt
left a comment
antoine-levitt
left a comment
There was a problem hiding this comment.
Looks nice! I just have an issue with the manifold selection API. Needs to be clearer that one atomic orbital = one Hubbard term = one l. I would suggest just
struct Hubbard
U
element::Element
atomic_orbital::String
end
struct TermHubbard
U
P
end
and getting rid of the manifold thing entirely. If you want to +U only selected atoms, then just make them different element types.
src/terms/hubbard.jl
Outdated
| stating whether the orbital belongs to the manifold. | ||
| """ | ||
| @kwdef struct OrbitalManifold | ||
| iatom ::Union{Int64, Nothing} = nothing |
There was a problem hiding this comment.
Why is this just an int? If I want to put a +U on two atom types, should I have two hubbard terms?
There was a problem hiding this comment.
At the moment it is not possible to have more than one +U correction, but I have a different version of the code which allows "multi-U" calculations with different atom types. I was planning to add it as a different PR once this one is closed.
There was a problem hiding this comment.
I think since it very much determines the structure of the code we should do it now
src/terms/hubbard.jl
Outdated
| where n or ibnd is the band index, ``weights[ik ibnd] = kweights[ik] * occupation[ik, ibnd]`` | ||
| and ``Pᵢₘ₁`` is the pseudoatomic orbital projector for atom i and orbital m₁ | ||
| (usually just the magnetic quantum number, since l is usually fixed). |
There was a problem hiding this comment.
This is not a "usually" it's enforced in the code, right? So it's actually enforced that each Hubbard term only deals with one type of atomic orbital. This needs to be enforced by the design.
There was a problem hiding this comment.
In the code this is enforced, yes, because the symmetrization of the nhubbard is l-dipendent.
mfherbst
left a comment
mfherbst
left a comment
There was a problem hiding this comment.
Thanks @fsicignano for updating this so quickly. I think it indeed makes for a cleaner code.
|
There is one more thing: currently the example takes a few minutes to run, maybe its discretization should be made coarser. |
Yes, I agree. If we can, we should. But with magnetism that's sometimes not possible. |
|
I think we are good now! |
|
@antoine-levitt now is the time if you want to have another look :) |
|
LGTM, thanks for doing this! |
4 participants
New Hubbard U term implementation.
Key features: - New hubbard.jl program
- New operator added in operators.jl
- OrbitalManifold struct has been moved to hubbard.jl
- self_consistent_field.jl has been updated to store the Hubbard n matrix and the occupation
- Hubbard term has been added to the tests in hamiltonian_consistency
- The pseudopotential used in hamiltonian_consistency has been changed with one having atomic orbitals